Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix cross track point perturbation #122

Merged
merged 3 commits into from
Dec 4, 2023
Merged

Conversation

SorooshMani-NOAA
Copy link
Collaborator

Fixes #118

@SorooshMani-NOAA
Copy link
Collaborator Author

@WPringle I think the failed tests are due to wrong references. I'll do more testing on all the 3 storms from NHC, but can you check if you see anything wrong with the changes I made?

@WPringle
Copy link
Contributor

WPringle commented Dec 2, 2023

@SorooshMani-NOAA Thanks for fix! Although it's strange since alpha already looks like it should be positive always and reason for not needing abs() value.

@SorooshMani-NOAA
Copy link
Collaborator Author

Although it's strange since alpha already looks like it should be positive

@WPringle I actually realized that later, but just wanted to be extra sure, the main fix was related to how 1 dim arrays where being incorrectly meaned or diffed

@WPringle
Copy link
Contributor

WPringle commented Dec 2, 2023

Although it's strange since alpha already looks like it should be positive

@WPringle I actually realized that later, but just wanted to be extra sure, the main fix was related to how 1 dim arrays where being incorrectly meaned or diffed

oh right, gotcha. think everything looks good

@SorooshMani-NOAA SorooshMani-NOAA self-assigned this Dec 2, 2023
Copy link
Contributor

@WPringle WPringle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if things are fixed without adding abs() around alpha then maybe don't make this change as it makes you think this is the bug but it's actually the other lines..

@codecov-commenter
Copy link

codecov-commenter commented Dec 4, 2023

Codecov Report

Attention: Patch coverage is 0% with 3 lines in your changes missing coverage. Please review.

Project coverage is 21.29%. Comparing base (eede830) to head (5b7c4e4).
Report is 14 commits behind head on main.

Files Patch % Lines
ensembleperturbation/perturbation/atcf.py 0.00% 3 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main     #122   +/-   ##
=======================================
  Coverage   21.29%   21.29%           
=======================================
  Files          28       28           
  Lines        3762     3762           
=======================================
  Hits          801      801           
  Misses       2961     2961           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@SorooshMani-NOAA SorooshMani-NOAA merged commit 1fbaf14 into main Dec 4, 2023
8 checks passed
@SorooshMani-NOAA SorooshMani-NOAA deleted the bugfix/crosstrack branch December 4, 2023 16:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Perturbation bug on turn to NE
3 participants